Skip to content

feat(dom): DOM - toHaveFocus #151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(dom): DOM - toHaveFocus #151

wants to merge 1 commit into from

Conversation

fonsiher
Copy link
Contributor

Added the toHaveFocus matcher with its tests.

const error = new AssertionError({
actual: this.actual,
expected: document.activeElement,
message: "Expected the element to have focus.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about the messaging to be:
"Expected the element to be focused" ?

const invertedError = new AssertionError({
actual: this.actual,
expected: document.activeElement,
message: "Expected the element not to have focus.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also in here, what do you think about the messaging to be:
"Expected the element NOT to be focused" ?

Comment on lines +288 to +291
const input1 = getByTestId("input1");
const input2 = getByTestId("input2");
input1.focus();
const test = new ElementAssertion(input2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a suggestion.
If you dont add .focus() , any element is focused so, I think the test could be easier like

Suggested change
const input1 = getByTestId("input1");
const input2 = getByTestId("input2");
input1.focus();
const test = new ElementAssertion(input2);
const input1 = getByTestId("input1");
const test = new ElementAssertion(input1);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants